fix(bind): restore []string values for map[string]interface{} duplicates#2982
fix(bind): restore []string values for map[string]interface{} duplicates#2982leno23 wants to merge 1 commit into
Conversation
When binding multipart/form-data to map[string]interface{}, store a single
value as string and multiple values as []string. This matches the behavior
before v4.13.0 and the intent of labstack#2656.
Fixes labstack#2731
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Heads-up for whoever reviews this: binding This PR re-introduces the slice behavior for the duplicate-key case, so it would re-break the same users #2656 was protecting. That may well be the right call for v5 (a major version is the place to change it) — just flagging that it's a deliberate backward-compat reversal, not a pure bugfix, so the decision is made consciously and noted in the v5 migration guide if accepted. |
There was a problem hiding this comment.
Pull request overview
Fixes a regression in Echo’s default binder where duplicate multipart/form-data fields bound into map[string]interface{}/map[string]any would incorrectly keep only the first value, restoring the intended behavior (single value as string, multiple values as []string).
Changes:
- Update
DefaultBinder.bindDatamap binding to store[]stringwhen a key has multiple values formap[string]interface{}. - Update existing map-binding expectations in
TestDefaultBinder_bindDataToMap. - Add a multipart/form-data regression test covering duplicate field names bound into
map[string]interface{}.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| bind.go | Restores multi-value binding behavior for map[string]interface{} when values are duplicated. |
| bind_test.go | Updates map-binding tests and adds a multipart regression test for duplicate fields. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } else if isElemInterface { | ||
| // To maintain backward compatibility, we always bind to the first string value | ||
| // and not the slice of strings when dealing with map[string]interface{}{} | ||
| val.SetMapIndex(reflect.ValueOf(k), reflect.ValueOf(v[0])) | ||
| if len(v) == 1 { | ||
| val.SetMapIndex(reflect.ValueOf(k), reflect.ValueOf(v[0])) | ||
| } else { | ||
| val.SetMapIndex(reflect.ValueOf(k), reflect.ValueOf(v)) |
|
This comment explains when and why this was changed #2731 (comment) I do not think this should be merged. |
|
Close it.
…On Mon, Jun 15, 2026 at 8:56 PM Martti T. ***@***.***> wrote:
*aldas* left a comment (labstack/echo#2982)
<#2982 (comment)>
This comment explains when and why this was changed #2731 (comment)
<#2731 (comment)> I
do not think this should be merged.
—
Reply to this email directly, view it on GitHub
<#2982?email_source=notifications&email_token=AACMVNENXNHOI654PCFNEZT5ADANDA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINZRGQ3TKOBYHEZKM4TFMFZW63VHMNXW23LFNZ2KKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-4714758892>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACMVNGVCO5HIMJGOFAUQOT5ADANDAVCNFSNUABEKJSXA33TNF2G64TZHMZTCNJQGQ2DSMJ3JFZXG5LFHM2DKMJTHA3TONRRG2QXMAQ>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Summary
map[string]interface{}stringand multiple values as[]stringProblem
Since v4.13.0, binding duplicate multipart fields like two
ima_slicevalues tomap[string]interface{}only kept the first value. Applications expecting a slice silently broke.This regressed after #2656, which intended to preserve pre-v4.12.0 single-string behavior but always bound
v[0].Fix
For
map[string]interface{}/map[string]any:string[]stringTest plan
TestDefaultBinder_bindDataToMapTestBindMultipartFormToMapInterfacego test ./... -run 'TestDefaultBinder_bindDataToMap|TestBindMultipartFormToMapInterface'Fixes #2731